Published on

让代码审查快 10 倍的思维模型

Authors
  • avatar
    Name
    俞凡
    Twitter

本文深入探讨了高效代码审查的思维模型，通过聚焦风险，提升审查效率，并且有助于高效缓解项目风险。原文：The Mental Model That Made Code Reviews 10x Faster

不要追求完美，而是关注风险

以前代码审查总让我筋疲力尽。一个 PR 要花 30 分钟，留下 15 条评论，却还是觉得漏掉了什么重要问题。提交者越来越抵触审查，我也累得不行，完全没效率可言。

后来一位技术主管教了我另一种审查思路，用一个问题改变了一切。

我不再问「这段代码有什么问题？」，而是问「这个 PR 里风险最高的地方是什么？」

突然间，代码审查从 30 分钟缩短到了 10 分钟。评论少了，但每条都切中要害。奇怪的是，提交者开始感谢我的审查，不再跟我争论。

完美审查的问题

我以前总想找出所有问题：每个风格问题、每个潜在改进、每个微小优化。审查清单通常是这样的：

  • 「这个变量应该用 const，不该用 let」
  • 「考虑把这块提取成辅助函数」
  • 「小问题：这里多了空格」
  • 「这段可以写成更函数式的风格」
  • 「你有没有考虑用 Map 代替对象？」
  • 还有 12 条评论……

你猜结果怎么着？提交者要么忽略大部分反馈，要么花几个小时处理鸡毛蒜皮，反而把我埋在第 8 条评论里的真正 bug 给漏掉了。

问题不在于我挑错了，而在于我审查时把所有问题都看得同等重要。

事实上它们根本不一样。

基于风险的框架

这个思维模型是：每个代码变更都有风险等级。你的工作是识别并解决风险，而非追求完美。

致命风险 —— 很可能导致生产环境问题：

  • 安全漏洞
  • 可能造成数据损坏
  • 竞态条件
  • 关键路径缺少错误处理
  • 破坏公共 API 的变更

高风险 —— 可能导致生产环境问题：

  • 大规模场景下的性能问题
  • 缺少数据库迁移
  • 业务逻辑错误
  • 破坏现有功能

中风险 —— 可能导致问题或技术债务：

  • 错误信息不清晰
  • 缺少调试日志
  • 代码难以维护
  • 过度耦合会影响未来变更

低风险 —— 代码风格和微小改进：

  • 变量命名
  • 代码组织
  • 微小性能优化
  • 遵循模式保持一致性

我以前的审查全都是低风险评论，纯粹是吹毛求疵。

转变：我从致命风险和高风险开始看。如果找到了这些问题，在解决之前，其他一切都不重要。

3 分钟扫描法

现在我首先会做一个快速的 3 分钟扫描，专门寻找暗示高风险的特定模式：

没有迁移的数据库变更：

// 红旗：模式更改，但没有迁移
class User {
  email: string;
  emailVerified: boolean;  // 新字段，迁移在哪里？
}

没有错误处理的外部 API 调用：

// 红旗：没有重试，没有超时，没有错误处理
async function syncUser(id) {
  const data = await externalAPI.getUser(id)
  return db.users.update(id, data)
}

认证/授权变更：

// 红旗：认证逻辑修改
if (user.role === 'admin' || user.isOwner) {  // 这里以前是'与'吗？
  allowAccess();
}

可能处理大量数据的数组操作：

// R红旗：没有分页，可能有数百万条记录
const users = await db.users.findAll();
return users.map(formatUser);

如果发现任何上述问题，审查就只关注这个，其他问题都先放一放。

两评论规则

我只允许自己留两种评论：

1. 阻塞评论 —— 合并前必须解决：「如果两个用户同时编辑，这会导致数据丢失。我们需要乐观锁。」

2. 仅供参考 —— 需要知道，但不阻塞合并：「FYI: 我们在 utils.js 里有一个类似的函数，对空值的处理方式不同，可以考虑对齐一下。」

没有「考虑一下」这种评论，没有「也许」评论，没有「你怎么看？」这种评论。

要么重要到需要阻塞合并，要么就只是提供信息。

这迫使我必须有明确立场。如果我都不愿意为此阻塞 PR，这事儿真有那么重要吗？

那个被漏掉的 bug 的故事

这个方法曾经救过我们一次。我当时在审查一个支付处理的 PR，改动了 200 多行代码。作者做了很多重构：重命名变量、提取函数。有很多地方可以评论。

放在以前，我会留下 20 条关于代码风格和组织的评论。

但现在，我做了 3 分钟扫描，发现了这个：

async function processRefund(orderId, amount) {
  const order = await getOrder(orderId)
  await stripe.refund(order.chargeId, amount)
  await db.orders.update(orderId, { status: 'refunded', refundedAmount: amount })
  return { success: true }
}

看起来没问题，对不对？但风险在这里：如果 Stripe 调用成功，但数据库更新失败怎么办？

我们已经退了款，但数据库不知道。客户拿到了钱，我们却没有记录。这就是致命风险。

我的整个审查就一条评论：「如果 Stripe 退款成功后数据库更新失败，我们会亏钱。需要用事务处理或者添加对账机制。」

提交者用分布式事务包装修复了这个问题，添加了幂等性，还加了对账任务。

一条评论，就可能避免了数千美元的退款损失。

那个 PR 的其他一切 —— 变量命名、函数提取、代码组织 —— 根本不重要。

我现在不再评论的内容

我不再评论这些：

linter 能抓到的代码风格问题。 如果团队用了 Prettier 或 ESLint，相信它们，不要做人类 linter。

主观改进。「这段可以更函数式」这种话毫无帮助。要么有具体问题，要么就没有问题。

没有数据支撑的性能优化。 除非你能证明这是瓶颈，否则优化都是过早的。

过度的抽象争论。 如果代码能用且清晰，抽象层级就没问题。

个人偏好。 我更喜欢 Map 而不是对象。挺好。但如果别人用对象，也完全没问题。

放下这些，我的审查时间减半，而且审查结果有用多了。

「LGTM」的惊人力量

当 PR 没有重大风险时，我开始只用「LGTM」批准。

一开始，我觉得这样不对。我总得找点什么评论来体现我的价值吧？

但结果是：PR 合并速度变快了。作者能更快得到反馈。而且当我真的留下评论时，他们开始相信这些评论确实重要。

上个月我给出的最棒的审查就是一句话：

「LGTM —— 实现干净，测试覆盖率好，没有明显风险。」

这个 PR 一小时后就合并了。没有来回拉扯，没有无谓争论，直接上线。

相比之下，一周前我在一个类似 PR 上留了 12 条小评论，结果我们争论变量名称就花了三天才合并。

让一切聚焦的问题

写任何评论之前，我都会问：「如果保持原样上线，最糟会发生什么？」

如果答案是：

  • 「生产环境崩溃」 → 阻塞 PR
  • 「我们会在凌晨 2 点被叫醒」 → 阻塞 PR
  • 「静默数据损坏」 → 阻塞 PR
  • 「用户会看到错误」 → 阻塞 PR
  • 「代码稍微没那么干净」 → 批准合并

这就过滤掉了我以前会评论的 80% 内容。

什么时候需要彻底审查

有些时候还是需要慢下来，一丝不苟：

对安全敏感的代码。 认证、支付、个人身份信息处理 —— 这些需要深度审查。我会追踪每一条路径，检查每一处验证，确认每一个权限检查。

核心基础设施变更。 数据库 schema 变更、API 契约变更、部署配置 —— 这些影响方方面面，值得仔细审查。

新团队成员写的代码。 不是因为他们技术不行，而是因为他们还不了解我们的坑。这是教学时间。

看不懂的变更。 如果你理不清逻辑，这就是一个信号。要么请求澄清，要么让他们简化。

但普通功能开发？认证中间件重构？修复管理后台的 bug？快速扫描风险，批准，继续下一个。

引以为傲的一次审查

有人提交了一个 PR，要用 WebSocket 添加实时更新。500 多行新代码。放在以前，我会花一个小时逐行检查。

这一次，我用 3 分钟扫描聚焦于：

  1. 连接断开了怎么处理？
  2. 服务器重启会发生什么？
  3. 消息保序吗？
  4. 我们会持久化任何内容吗？
  5. 故障模式是什么？

找到了两个致命问题：

「如果服务器重启，客户端不会重连。它们就会一直停在那里显示过期数据。我们需要带指数退避的自动重连。」

「消息没有持久化到任何地方。如果客户端消息到达时不在线，他们就永远看不到了。我们需要消息队列或事件日志。」

这些都是阻塞问题。其他一切 —— 代码结构、命名、模式 —— 都没问题。

审查花了 15 分钟，避免了两起生产事故。

模式识别游戏

做了几百次审查后，你会开始立刻识别风险模式：

「碰了数据库，碰了外部 API」 → 检查事务边界和故障处理。

「新增环境变量」 → 检查是否有文档，是否有合理的默认值。

「修改错误处理」 → 检查错误是否仍然被日志记录/监控。

「并发原语（锁、原子操作、通道）」 → 检查死锁和竞态条件。

「日期/时间处理」 → 检查时区处理。

「删除代码」 → 检查是否真的没人用，会不会破坏功能。

「配置变更」 → 检查向后兼容性。

这种模式匹配让审查变得飞快。我不需要逐行阅读，只需要扫描我知道有风险的模式。

好审查的标准

我最近做过的最好的代码审查都有这些共同点：

  • 一到两条聚焦的评论，而不是一长串清单
  • 清晰解释风险，而不只是说「这错了」
  • 尽可能给出具体建议
  • 快速反馈 —— 当天完成审查，通常几小时内
  • 信任作者 —— 假设对方能力足够，不故意找茬

说实话，最好的审查往往就是一句：「看起来不错，上线吧。」

实践中的思维模型

我现在实际的审查流程是这样的：

1. 阅读描述（1 分钟） 这是要做什么？为什么要做？这个区域的风险等级是什么？

2. 扫描关键模式（2 分钟） 数据库变更？API 调用？认证？并发？外部依赖？

3. 必要时深入（5-10 分钟） 如果发现风险，彻底理解它。追踪代码路径，想清楚各种故障模式。

4. 评论或批准（1 分钟） 要么清晰说明理由阻塞合并，要么批准继续下一个。

总计：根据复杂度和风险，每次审查 5-15 分钟。

对比以前的我：每次 30-45 分钟，留下 15 条评论，大部分都不重要。

思维转变

思维转变就是：不要再追求代码完美，而是努力防止事故。

你的工作不是抓到所有问题，而是抓到重要问题。

代码审查不是为了代码质量，而是为了缓解风险。

当我明白这一点后，审查变得轻松多了，也有效多了。

现在我打开 PR，只会问一个问题：「这里风险最高的是什么？」

其他一切都是噪音。